fix: overloading behaviour #14
Conversation
|
Ok I'm unsure what I did wrong. Maybe this should have only been changed for array access. (I only ran tests and not phpstan locally, my bad!) |
|
@compwright have you looked into this? |
There was a problem hiding this comment.
@yani tests continue to fail even after removing clone. Please fix the test failures, and I will be happy to merge this in.
|
Hi. Sorry for taking so much time. I might have some time tomorrow. I'll have to look for a way to make sure it's tested correctly. I think the test case should just be that it doesn't throw an exception, which I think is correctly handled by PHPUnit which would just return false for the case if it does. I still have to look at any possible differences between using the dynamic property way of doing it. |
|
I had to manually throw E_USER_NOTICE to keep current tests working but also implement the original overloading behavior. I tested it on the project where I encountered the error and everything seems to be working as intended as far as I can tell. |
|
@compwright I'm not sure if you're waiting for me to mark your change request as done. But I'm unable to mark it as such. If you're keeping this open because you don't like the E_USER_NOTICE, I could maybe try and figure out another way but I think this is as good as it gets. |
This code:
Resulted in the following error:
This PR makes the magic getter methods act as references.
It will behave like a normal array then.
The reason I added
clonein the read tests is because I think the original PHP error is thrown when the variable is actually being used instead of just being referenced. "Using" the variable withclonefixes the test.I think I understand everything correctly and fixed this. You can see how the tests fail if you remove the reference tokens (
&) from the methods.